Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change TftSpiBase displays to use SpiBus.Write instead of Exchange #1122

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

HakanL
Copy link
Contributor

@HakanL HakanL commented Feb 13, 2025

Fix for WildernessLabs/Meadow_Issues#826

Also changed API for ST7789 to have the reset pin optional (nullable) (it is optional when driving the display, so it should be nullable IMO)

Changed API for ST7789 to have the reset pin optional
@HakanL
Copy link
Contributor Author

HakanL commented Feb 14, 2025

@ctacke what is your take on the proper way to do SpiBus.Exchange on a FT232H? It seems risky to have a spinning loop expecting data to be queued up after the full read is completed, the purpose of Exchange on SPI is to do a concurrent write and read. Also questionable if a large write (the display I'm testing with, ST7789, is sending 48k for a full update) would even have that much available space for a read queue?

@ctacke
Copy link
Contributor

ctacke commented Feb 17, 2025

I'd agree that spinning waiting is risky, but I don't see a lot of other options. It probably needs a timeout, but right now just getting "working" for both SPI and I2C with the same driver eludes me, and I've gone back and rewritten this code several times trying to get it to behave.
I know the device has a buffer limit - I don't recall what it is - but it would not surprise me if 48k is too large for a single transfer. I would thing that we could just packetize since it's synchronous serial and pauses while queue new buffers shouldn't cause the client any grief since it's only looking at the clock signal. As long as we leave the CS active it should just work.

@HakanL
Copy link
Contributor Author

HakanL commented Feb 17, 2025

I read that you need to tie D0 and D1 together to activate I2C mode on the FT232H, perhaps that's causing issues for you? Yeah, but I believe the driver takes care of cutting up the buffer in blocks, we just seem to call it with the full 48k buffer as a parameter, so unless we split up into let's say 4k blocks in the C# code it may not be possible. It's also odd that the Exchange wouldn't have anything in the input buffer, unless I'm mistaken, during Exchange the data is read concurrently from MISO, using the same clock signal that is used for output, so in theory there should always be data (but it will be 0s) when doing an Exchange, even with nothing connected. Ie I don't believe there is a way for the SPI bus to know if something is sending anything back, or if's all 0s, it's up to the driver to use the content or not.

@adrianstevens adrianstevens changed the base branch from develop to test_tft_changes February 17, 2025 19:35
@adrianstevens
Copy link
Contributor

merging into a temp branch so we can do performance testing before merging to develop

@adrianstevens adrianstevens merged commit 2ebf06c into WildernessLabs:test_tft_changes Feb 17, 2025
1 check passed
@HakanL
Copy link
Contributor Author

HakanL commented Feb 17, 2025

I wonder if it would be safe to do a call for GetAvailableBytes right after the send, and if it's 0 then we don't even go into the read loop? What do you think? If there's really an exchange happening then there will be data in the buffer (at least that's my assumption since the read and write is concurrent, but I'm not sure if there is a delay in getting that data into the FT232H, or over USB that could case problems for short write (1 byte). That's outside my area, I'm just concerned with the read loop, even with a timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants